Skip to content

fix: O3-5419 Add comprehensive error handling for sessionStorage operations#585

Open
sanks011 wants to merge 4 commits intoopenmrs:mainfrom
sanks011:O3-5419-bug-unhandled-session-storage-errors-crash-search-history-component
Open

fix: O3-5419 Add comprehensive error handling for sessionStorage operations#585
sanks011 wants to merge 4 commits intoopenmrs:mainfrom
sanks011:O3-5419-bug-unhandled-session-storage-errors-crash-search-history-component

Conversation

@sanks011
Copy link

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

This PR fixes a critical bug where the search history component crashes when sessionStorage contains corrupted JSON data or when the storage quota is exceeded.

Problem

The search history functionality used sessionStorage without error handling, causing application crashes in production when:

  • SessionStorage contains invalid/corrupted JSON data (browser bugs, extensions, manual tampering)
  • SessionStorage quota is exceeded (~5MB limit across all browsers)
  • No graceful degradation - entire search history feature becomes broken

Solution

Implemented comprehensive error handling with:

File 1: src/components/search-history/search-history.utils.ts

  • Added try-catch blocks around JSON.parse() operations
  • Added data validation to ensure parsed data is an array
  • Automatically clears corrupted sessionStorage data
  • Returns empty array on errors (graceful degradation)

File 2: src/cohort-builder.utils.ts

  • Implemented LRU cache (maximum 50 items) to prevent unbounded growth
  • Added QuotaExceededError handling with fallback to 10 most recent items
  • Limited patient data to 100 per search to prevent storage bloat
  • Added user notifications via showToast for quota errors
  • Input validation for function parameters
  • Comprehensive JSDoc documentation

Impact

  • Prevents complete application crashes from corrupted storage data
  • Handles quota exceeded scenarios gracefully with user notification
  • Production healthcare facilities performing 100+ searches/day no longer hit limits
  • Searches continue working even if history storage fails (graceful degradation)

Testing

  • All existing tests pass (25/26 - one pre-existing flatpickr locale failure unrelated to changes)
  • Manually tested with corrupted sessionStorage data - no crash, graceful recovery
  • Console logging added for debugging without alarming end users

Screenshots

Before Fix:

OpenMRS.-.Brave.2026-02-11.13-34-13.mp4
  • App crashes with red error overlay: SyntaxError: Expected property name or '}' in JSON at position 1
  • Search history component completely broken
  • No recovery without manually clearing browser data

After Fix:

OpenMRS.-.Brave.2026-02-11.13-47-00.mp4
  • Corrupted data detected and cleared automatically
  • Console log for debugging: [Cohort Builder] Corrupted history data, resetting
  • App continues functioning normally
  • User sees toast notification only for actionable issues (quota exceeded)

Related Issue

https://issues.openmrs.org/browse/O3-5419

Other

Reproduction Steps (Before Fix)

  1. Open browser console
  2. Run: window.sessionStorage.setItem('openmrsHistory', '{invalid json}');
  3. Navigate to Cohort Builder or perform any search
  4. Result: Application crashes

After Fix Behavior

  1. Same corruption applied
  2. Navigate to Cohort Builder or perform any search
  3. Result: Console log shows error was caught, data cleared, app works normally

Technical Details

  • No breaking changes
  • Backward compatible with existing sessionStorage data format
  • TypeScript and ESLint compliant
  • Follows React best practices for error handling
  • Implements best practices from MDN Web Docs for Web Storage API

…ations

- Add try-catch blocks around JSON.parse() in search-history.utils.ts
- Implement LRU cache (max 50 items) to prevent unbounded growth
- Add QuotaExceededError handling with fallback to 10 items
- Validate input data and parsed JSON structure
- Limit patient data to 100 per search to prevent storage bloat
- Add user notifications via showToast for storage errors
- Clear corrupted sessionStorage data automatically
- Add JSDoc documentation for error handling behavior

Fixes crash when sessionStorage contains corrupted JSON or quota is exceeded.
Implements graceful degradation - searches continue working even if history fails.
@UjjawalPrabhat
Copy link

@sanks011 The core error handling is solid, but there are some concerns that need addressing:

  1. Your PR fixes the vulnerability in two files but misses a third location with the same issue: src/components/composition/composition.utils.ts

  2. The codebase currently has zero console.log/warn/error statements - errors are handled silently or shown to users via showSnackbar. Your PR adds 6+ console statements. Remove all console statements, or keep at most 1-2 for truly exceptional cases. We should rely on user-facing notifications instead of console logging.

…builder-app into O3-5419-bug-unhandled-session-storage-errors-crash-search-history-component

st ":wq" | clip ; Write-Host "Type :wq and press Enter in the editor to
complete the merge"
…console statements

- Add error handling to composition.utils.ts (third file with same bug)
- Remove all 9 console.log/warn/error statements (codebase has zero console logs)
- Keep only user-facing showToast notifications for actionable errors
- Silent error recovery for corrupted storage data
@sanks011 sanks011 force-pushed the O3-5419-bug-unhandled-session-storage-errors-crash-search-history-component branch from 067cac6 to aecd2a0 Compare February 12, 2026 05:54
@sanks011
Copy link
Author

@sanks011 The core error handling is solid, but there are some concerns that need addressing:

  1. Your PR fixes the vulnerability in two files but misses a third location with the same issue: src/components/composition/composition.utils.ts
  2. The codebase currently has zero console.log/warn/error statements - errors are handled silently or shown to users via showSnackbar. Your PR adds 6+ console statements. Remove all console statements, or keep at most 1-2 for truly exceptional cases. We should rely on user-facing notifications instead of console logging.

@UjjawalPrabhat Thanks for the feedback! I've addressed both concerns:

  1. Fixed composition.utils.ts - Added comprehensive error handling with try-catch, data validation, and silent recovery
  2. Removed all console statements - Deleted all 9 console.log/warn/error calls. Now using only user-facing showToast notifications for actionable errors

Changes pushed. Please let me know if any further changes are required.

@bhavya-jpg
Copy link

As @UjjawalPrabhat pointed out, src/components/composition/composition.utils.ts also touches sessionStorage and therefore runs the exact same risk of throwing quota errors or reading corrupted JSON. To prevent duplicating these large try/catch, Array.isArray(), and JSON.parse blocks across three different codebases, would it make sense to extract this into an app-level utility? (e.g., creating a safelySetSessionStorage(key, value) and safelyGetSessionStorage(key) helper function). Building a shared utility wrapper would protect the entire app universally and cover composition.utils.ts cleanly without redundant code.

@bhavya-jpg
Copy link

@sanks011 and one more tiny thing also in your addToHistory function, you process inputs with: if (!Array.isArray(patients) || typeof parameters !== 'object') { return false; } Just a quick heads up: in Javascript, typeof null === 'object'. Therefore, if parameters somehow gets accidentally passed as null by an upstream function, it will bypass this check and potentially cause null-reference errors downstream when you serialize it. We can easily plug this leak by strictly adding a null check: if (!Array.isArray(patients) || typeof parameters !== 'object' || parameters === null) { return false; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants